@bzbarsky-apple Looks like a real build failure

the new code also creates spans using GetLength() as the previous code

But only after calling a method (GetDataPtr, see comment on that line) that validates that the length is at least sane in the sense of not overrunning the available buffer. That is a key difference.

these methods could work on TLVReader as before

Sure thing. If you know enough about the state of your reader. The new setup just delegates that state verification to the compiler, which is less error-prone than having humans do it.

just call GetDataPtr/GetLength in the places in setup payload as needed (stop allocating temporary storage space)

Yes, we could absolutely do that. The drawbacks there are that it's non-obvious code to write, so people are not likely to write it by default. Instead they reach for GetString/GetBytes, which have a nicer API and very non-obvious pitfalls.

I initially started with an attempt to just remove GetString and GetBytes altogether, because they are so easy to misuse. That ran into problems with the non-contiguous reader cases, where GetString and GetBytes cannot be easily replaced with in-place views because the data is not all there all at once. I am still thinking about what we can actually do with GetString and GetBytes to make them safe to use, if it's even possible in general, but decided to not block on solving that problem before fixing the memory-safety issues we have with their current consumers....

I am open to other API shapes here, by the way; what we have so far is not perfect by any means. It's a bit of a compromise between expediency and redesigning the entire TLV reader setup wholesale.

Approved based on comments. I wish we had some resources to make TLV better - I think it went too far into trying to optimize backing store at the expense of safety and I am not convinced the fancy backing store is actually that used.

@msandstedt @saurabhst @LuDuda @jepenven-silabs @jmartinez-silabs Please take a look?

Size increase report for "esp32-example-build" from 488a3b0

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_info,0,1555
.debug_str,0,891
.debug_line,0,561
.debug_frame,0,72
.debug_ranges,0,48
.flash.text,48,48
.debug_abbrev,0,26
.debug_aranges,0,24
.debug_loc,0,3
[Unmapped],0,-48

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,1555
.debug_str,0,893
.debug_line,0,551
.debug_loc,0,363
.debug_frame,0,72
.flash.text,60,60
.debug_ranges,0,48
.debug_abbrev,0,26
.debug_aranges,0,24
[Unmapped],0,-60

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,1555
.debug_str,0,883
.debug_line,0,561
.debug_loc,0,267
.debug_frame,0,72
.flash.text,56,56
.debug_ranges,0,48
.debug_abbrev,0,26
.debug_aranges,0,24
[Unmapped],0,-56

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_info,0,1555
.debug_str,0,891
.debug_line,0,561
.debug_loc,0,219
.debug_frame,0,72
.flash.text,68,68
.debug_ranges,0,48
.debug_abbrev,0,26
.debug_aranges,0,24
[Unmapped],0,-68

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,1340
.debug_str,0,892
.debug_line,0,415
.debug_frame,0,100
.debug_loc,0,58
.debug_ranges,0,48
.debug_abbrev,0,26
.debug_aranges,0,24
.riscv.attributes,0,1